Skip to content

Configurable AI model selection with automatic fallback (fixes #91)#102

Open
tbartik wants to merge 1 commit into
microsoft:mainfrom
tbartik:feat/configurable-ai-model-selection
Open

Configurable AI model selection with automatic fallback (fixes #91)#102
tbartik wants to merge 1 commit into
microsoft:mainfrom
tbartik:feat/configurable-ai-model-selection

Conversation

@tbartik

@tbartik tbartik commented Jun 2, 2026

Copy link
Copy Markdown

Description

The extension's AI features (skill generation, quizzes, code reviews, did-you-know, rule compilation, etc.) previously selected a single Copilot chat model by family (gpt-5.4-minigpt-5-minigpt-4.1-minigpt-4.1, then first-available). When that model is disabled for the user's Copilot plan or organization, it is still selectable via vscode.lm.selectChatModels but returns an empty response. The empty text then fails JSON parsing, so AI features error out — or silently produce nothing — with no actionable guidance for the user.

This PR makes model selection configurable and resilient:

  • New src/core/llm-models.ts — a single source of truth for model selection:
    • New aiEngineerCoach.preferredModel setting plus an in-memory runtime override (so a change applies to the next request immediately and persists across reloads).
    • Auto mode orders available models by a vendor-neutral capability heuristic (scoreFamily) and iterates them, transparently skipping any model that returns an empty response instead of relying on a single pick.
    • De-duplicates models by id (Copilot can return the same model multiple times).
    • Pure helpers (scoreFamily, orderModelsByPreference, dedupeModelsById) are unit-tested.
  • panel-llm.tscallLlm/callLlmJson iterate candidate models and skip empty responses. When every model returns empty, they throw a clear message pointing the user at the new "AI Model" picker. Existing JSON-repair (balanceTruncatedJson) and structured-output fallback behaviour are preserved.
  • Dashboard sidebar — adds an "AI Model" dropdown bound to the setting via new listModels / setModel RPCs (panel-html.ts, app.ts, panel-request-service.ts, rpc-types.ts).
  • rule-compiler.ts — uses the shared candidate list instead of a hardcoded gpt-4.1 family, so rule compilation benefits from the same fallback.

The capability ordering is deliberately organization-neutral: it does not assume which models any particular plan has enabled — a disabled model simply returns empty and the loop falls through to the next candidate.

Related Issues

Fixes #91

Checklist

  • npm run check passes (typecheck + lint + spellcheck + knip + tests)
    • Note: one pre-existing, environment-sensitive test (src/core/parser-vscode.test.tsfindVsCodeDirs) fails on a clean main checkout as well; it is unrelated to this change.
  • Changes are covered by tests (if applicable) — src/core/llm-models.test.ts (11 cases for scoring, ordering, and de-duplication)
  • Documentation updated (if applicable) — the new setting is self-documenting via its markdownDescription in package.json

…crosoft#91)

The extension's AI features previously selected a single Copilot chat model
by family (gpt-5.4-mini -> ... -> gpt-4.1). When that model is disabled for
the user's Copilot plan or organization it stays *selectable* but returns an
empty response, so JSON parsing fails and AI features error out (or silently
produce nothing) with no actionable guidance.

This adds a central model-selection module and routes every AI feature
(panel LLM helpers and the rule compiler) through it:

- core/llm-models.ts: single source of truth for the preferred model
  (new aiEngineerCoach.preferredModel setting + in-memory runtime override).
  Auto mode orders candidates by a vendor-neutral capability heuristic and
  iterates them, transparently skipping any model that returns an empty
  response. Pure helpers (scoreFamily, orderModelsByPreference,
  dedupeModelsById) are unit tested.
- panel-llm.ts: callLlm/callLlmJson iterate candidate models and skip empty
  responses; when every model returns empty they throw a clear message that
  points the user at the new "AI Model" picker. Upstream JSON-repair and
  structured-output handling are preserved.
- Dashboard sidebar gains an "AI Model" dropdown bound to the setting via new
  listModels/setModel RPCs.
- rule-compiler.ts uses the shared candidate list instead of a hardcoded
  gpt-4.1 family.
@tbartik

tbartik commented Jun 2, 2026

Copy link
Copy Markdown
Author

Heads-up on overlap with #98 ("add Gemini CLI support and provider-agnostic AI settings").

Both PRs add a user-configurable preferred-model setting + a model picker, so they touch some of the same files (panel-llm.ts, panel-request-service.ts, panel-html.ts, app.ts, package.json). A couple of things worth calling out so maintainers can decide how to sequence them:

This PR is specifically the fix for #91. #98 refactors model selection (preferred id + a hardcoded high-capability family list) but still does a single selectModel() returning models[0] with no empty-response handling. The root cause of #91 is that a model disabled for the user's Copilot plan/org is still selectable yet returns an empty response — so the single-pick path keeps crashing. This PR instead iterates all candidate models and skips empty responses, surfaces an actionable error pointing at the model picker when every model is empty, routes the rule compiler through the same logic, and adds unit tests for the selection/ordering/dedup helpers.

Possible conflict: the setting key differs — #98 uses aiEngineerCoach.preferredModelId, this PR uses aiEngineerCoach.preferredModel. Happy to align on whichever name maintainers prefer.

I'm glad to rebase this on top of #98 (layering the empty-response fallback onto their selection logic) or vice-versa — whatever ordering is easiest to review. The Gemini CLI parser in #98 is independent and valuable on its own.

@tbartik

tbartik commented Jun 2, 2026

Copy link
Copy Markdown
Author
@microsoft-github-policy-service agree

@microsoft-github-policy-service agree

@san360 san360 added the enhancement New feature or request label Jun 5, 2026
@san360 san360 requested review from aymenfurter and Copilot June 20, 2026 04:14

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the extension’s Copilot chat model selection configurable and resilient by centralizing model selection, adding a dashboard “AI Model” picker, and iterating through candidate models to avoid failures caused by models that are selectable but return empty responses (e.g., disabled by plan/org).

Changes:

  • Introduces src/core/llm-models.ts as a shared model-selection layer (preference + auto ordering + helpers + tests).
  • Updates LLM callers to iterate candidate models and skip empty responses; adds sidebar UI + RPC methods to list/select models.
  • Switches the natural-language rule compiler to use the shared candidate model list instead of a hardcoded family.

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/webview/panel-request-service.ts Adds RPC handlers (listModels, setModel) used by the dashboard model picker.
src/webview/panel-llm.ts Updates callLlm/callLlmJson to try multiple models and handle empty responses; re-exports model APIs.
src/webview/panel-html.ts Adds “AI Model” dropdown to the dashboard sidebar.
src/webview/app.ts Populates the model dropdown via RPC and persists selection changes.
src/core/types/rpc-types.ts Extends RPC type map with listModels / setModel.
src/core/rule-compiler.ts Uses shared candidate model list and falls through on empty responses.
src/core/llm-models.ts New centralized model selection + preference persistence + ordering/dedupe helpers.
src/core/llm-models.test.ts Adds unit tests for scoring, ordering, and deduplication helpers.
package.json Adds aiEngineerCoach.preferredModel setting with markdown description.
CHANGELOG.md Documents the new configurable model selection and fallback behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/core/llm-models.ts
Comment on lines +195 to +204
const all = await vscode.lm.selectChatModels({});
if (all.length === 0) {
throw new Error('No language model available. Make sure GitHub Copilot is installed and signed in.');
}
const preferred = getPreferredModelId();
const ordered = orderModelsByPreference(all, preferred);
runtimeDebug('llm', 'models-available',
`preferred=${preferred ?? '(auto)'} count=${ordered.length} ` +
`models=${ordered.map(m => `${m.id}(${m.family})`).join(',')}`);
return ordered;
Comment thread src/webview/panel-llm.ts
Comment on lines 418 to 419
const response = await model.sendRequest(retryMessages, options, cts.token);
for await (const chunk of response.text) text += chunk;
Comment thread src/core/rule-compiler.ts
Comment on lines +100 to +115
// Iterate candidates so a model that is disabled for the plan/organization
// (which returns an empty response) transparently falls through to the next.
for (const model of candidates) {
const response = await model.sendRequest(messages, {});
let result = '';
for await (const chunk of response.text) {
result += chunk;
}
if (result.trim().length === 0) continue;

// Extract markdown from code block if wrapped
const fenced = result.match(/```(?:markdown)?\s*\n([\s\S]*?)```/);
if (fenced) return fenced[1].trim();
if (result.includes('---')) return result.trim();
// Extract markdown from code block if wrapped
const fenced = result.match(/```(?:markdown)?\s*\n([\s\S]*?)```/);
if (fenced) return fenced[1].trim();
if (result.includes('---')) return result.trim();
return null;
}
Comment thread src/webview/panel-html.ts
</div>
<div class="sidebar-filter">
<label for="model-filter">AI Model</label>
<select id="model-filter"><option value="">Auto (first available)</option></select>
@aymenfurter

Copy link
Copy Markdown
Contributor

How about we try selectChatModels({ family: 'auto' }) instead? Copilot picks an enabled model for the user's plan and handles fallback itself. That might replace most of the ranking and fallback logic here. It would also avoid hardcoding model names that go stale over time. Worth a quick spike to compare?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error: LLM returned invalid JSON after 3 attempts. Please try again.

4 participants